-
Notifications
You must be signed in to change notification settings - Fork 153
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Custom Privacy Text Option to CTAs #12293
Add Custom Privacy Text Option to CTAs #12293
Conversation
…ng a cta, or provide empty string if default
…efault privacy text if no custom privacy text found
…t on petition pages
…duce duplicate code a bit
…:MozillaFoundation/foundation.mozilla.org into TP1-394-12169-add-privacy-text-to-signup
Sorry for the delay. I will review this PR today. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yay we are almost ready to enable A/B test for newsletter signup form! I've left some inline comments, please let me know what you think! After searching for all instances of newsletter-signup-module
, I've also noticed there are a few instances missing the data-cta-privacy-notice
attribute. For example, in bannered_campaign_page.html
around line 65, network-api/networkapi/templates/fragments/primary_nav.html
around line 85, and maybe some more. Could we please add the attribute to those places? Thanks!
network-api/networkapi/wagtailpages/templates/wagtailpages/fragments/formassembly_body.html
Outdated
Show resolved
Hide resolved
source/js/components/newsletter-signup/organisms/default-layout-signup.jsx
Outdated
Show resolved
Hide resolved
source/js/components/newsletter-signup/organisms/default-layout-signup.jsx
Outdated
Show resolved
Hide resolved
… quote for rich text filter
Thanks for the review and feedback @mmmavis! PR should be up-to-date. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@robdivincenzo R+! Awesome work! Before you merge this PR, could you please add a note at the top of wagtailpages/fragments/formassembly_body.html
about the changes you applied to the privacy field? Thank you.
➤ Simon Acosta Torres commented: PR has been merged. |
Description
This PR adds a field to CTAs to overwrite the default privacy notice. This new field will allow users to A/B test custom privacy text vs. the default privacy text on various signups or petitions.
When this field is set, it will display instead of the default privacy notice. When this field is empty (as by default), the default privacy text will display.
Link to sample test page:
Related PRs/issues: #12169 / TP1-394
Credentials:
username: admin
password: W5YO6CFsu0$98!ch
To Test
...
actions to edit the CtaChecklist
Tests
- [ ] Is the code I'm adding covered by tests?Changes in Models:
- [ ] Did I update or add new fake data?- [ ] Did I squash my migration?Documentation:
- [ ] Is my code documented?- [ ] Did I update the READMEs or wagtail documentation?Merge Method
💡❗Remember to use squash merge when merging non-feature branches into
main
┆Issue is synchronized with this Jira Story